H4HIP: Forward compatibility: `Chart.yaml` `minimumHelmVersion` field #370

gjenkins8 · 2024-11-19T06:01:35Z

No description provided.

marckhouzam

I like this

hips/hip-XXXX.md

mattfarina

I like the idea of a minimumHelmVersion field. I have some thoughts...

There is a capabilities property for the current version of Helm and it's possible to encode an error rendering into a template when you don't meet the minimum version. It's not as elegant as this.
Do we have some other new chart features so that we should have a new chart version? We are currently on chart apiVersion v2.
I'm questioning the required nature of this and when to keep it updated. Should this only be present if some new thing was introduced along the way? And, should it only have the minimum version for the oldest version that would support the chart? I'm thinking about end-users who use a WIDE range of Helm versions. There are so many old Helm versions in use and we aren't going to see this change.

mattfarina · 2024-11-21T14:58:35Z

hips/hip-XXXX.md

+The minor and patch fields are optional.
+And will be inferred as zeros if not specified.
+
+When loading a chart, iff the `minimumHelmVersion` exists, Helm will verify its version exceeds or equals the version constraint specified in the field.


iff -> if

This is one of my favorite contractions :) iff -> if and only if

https://www.merriam-webster.com/dictionary/iff

ie. the above reads like:

When loading a chart, if and only if the minimumHelmVersion exists, Helm will verify its version exceeds or equals the version constraint specified in the field.

I would think we would want to omit the use of abbreviations as the meaning may not be broadly understood, especially for those where English is not a first language

ok, fair. will drop usage of iff

mattfarina · 2024-11-21T14:59:06Z

hips/hip-XXXX.md

+
+When loading a chart, iff the `minimumHelmVersion` exists, Helm will verify its version exceeds or equals the version constraint specified in the field.
+
+When "strict" loading of `Chart.yaml` is used: ie. the loading of `Chart.yaml` errors when unknown fields are present, Helm will first 'peek' into `Chart.yaml` and extract the `minimumHelmVersion` field (iff it exists) and perform the version constraint check at this point.


iff -> if

mattfarina · 2024-11-21T15:21:48Z

hips/hip-XXXX.md

+While this may not be strictly necessary, it will provide a backstop for features that may be included in the chart by the user for their current Helm version or `helm create`.
+The user can remove/reduce the specified `minimumHelmVersion` if desired.
+
+`helm lint` will produce a warning, if the current version of Helm is newer than the minimum version constraint.


A warning if the current version is newer? Do you mean, if the current version is older?

I agree this is too aggressive.

I did originally mean newer, with the intention as follows. Will address in #370 (comment)

mattfarina · 2024-11-21T15:27:23Z

hips/hip-XXXX.md

+
+`helm lint` will produce a warning, if the current version of Helm is newer than the minimum version constraint.
+Or the `minimumHelmVersion` field is unset for the given chart.
+This is to encourage users to update the minimum version to currently utilized version of Helm (that the user is using to develop the chart).


We see a difference between people who create a chart and those who consume charts (see the profiles/roles). Consider all the charts people pull from repositories listed on Artifact Hub.

There is also a large variety of different versions of Helm in use. In our previous CDN we had data on this. While the majority were using a relatively new version, it was not unusual for people to use older version. Even those over 2 years old.

Even in those final reports, from just a few months ago, there were cases of people pulling 3.0.0 regularly. A bad idea but it's happening. Versions in use go further back than we would like.

If we want to make charts widely usable, do we want to have a relatively recent version of Helm set as the minimum version if there is no feature in a newer version of Helm (e.g., new template function) being used? What is the benefit?

I've removed helm lint reporting missing or old minimumChartVersion field. While users keeping Helm up to date would be good, this probably isn't the right way to "encourage" them to do so.

mattfarina · 2024-11-21T15:34:41Z

hips/hip-XXXX.md

+
+## Abstract
+
+This HIP proposes supporting a `minimumHelmVersion` field in `Chart.yaml`.


We already have a property called KubeVersion to specify a range for Kubernetes support. Should this be called HelmVersion and accept a range in the same way? The range syntax being the same used elsewhere in Helm.

Happy to call the field HelmVersion (naming is hard :) )

I don't think we should/need to support anything for than a single, implied to be the minimum version, constraint however. I've addressed this here:

https://github.com/helm/community/pull/370/files#diff-e94b50b2c7a2d45c81db342448ae78950464d77488a62b315a2b715549fe1a95R95

sabre1041

Looks good. A few comments

hips/hip-XXXX.md

sabre1041 · 2024-11-24T11:15:42Z

hips/hip-XXXX.md

+The minor and patch fields are optional.
+And will be inferred as zeros if not specified.
+
+When loading a chart, iff the `minimumHelmVersion` exists, Helm will verify its version exceeds or equals the version constraint specified in the field.


I would think we would want to omit the use of abbreviations as the meaning may not be broadly understood, especially for those where English is not a first language

hips/hip-XXXX.md

sabre1041 · 2024-11-24T11:22:28Z

hips/hip-XXXX.md

+When "strict" loading of `Chart.yaml` is used: ie. the loading of `Chart.yaml` errors when unknown fields are present, Helm will first 'peek' into `Chart.yaml` and extract the `minimumHelmVersion` field (iff it exists) and perform the version constraint check at this point.
+This is to provide better UX that simply erroring on any potential new fields in `Chart.yaml` that are unknown to the current version of Helm.
+
+`helm create` will pre-fill Chart.yaml's `minimumHelmVersion` field with the current version of Helm.


Should this new proposed parameter be added at chart instantiation? Other similar values, like KubeVersion are not added automatically

I've remove this. Goal was to keep new charts requiing new versions of Helm. But agree, this is too agressive.

The default starter chart should contain the minimumHelmVersion version field, if the chart requires that minimum version of Helm to operate.

hips/hip-XXXX.md

sabre1041 · 2024-11-24T16:34:40Z

hips/hip-XXXX.md

+However, since core Helm capabilities are only every additive due to Helm's [backwards-compatibility rules][4], a capabilities based system is redundant.
+
+*NB: at the time of writing, plugins for extending Helm's functionality are being heavily discussed.
+And while plugins will likely allow extending capabilities independent of the Helm version.


Combine with the following sentence? Its currently a fragment

oops, right

godhanipayal · 2025-01-30T18:22:13Z

hips/hip-XXXX.md

+But potentially an incompatibility may go undetected, and no hard error will be presented.
+
+While the hard-failure case is better, it still requires the user to debug and realize the failure is due to a version mismatch.
+The second "soft-failure" case is much worse, as it could lead to unexpected behavior in the deployed chart application, likely requiring a much more involved debugging requirement and confusing user experience.


I really like this idea. However, I was wondering—since Helm versions need to be compatible with the Kubernetes cluster, what happens if a newer Helm version isn't supported on a customer's Kubernetes version? Would this change effectively force customers to upgrade their Kubernetes cluster immediately in order to proceed with Helm deployments?
https://helm.sh/docs/topics/version_skew/

The premise behind the minimumHelmVersion field, is that the chart requires a feature present only in newer versions of Helm. If a user wants that chart, they need that newer version of Helm :)

If they also want an old version of Kubernetes, then yeah, they are perhaps out of luck. But two things can be said here:

minimumHelmVersion isn't changing the fact that a newer version of Helm is needed. It is just making the error upfront (as this section points out)

The version skew is the "guaranteed" compatibility matrix. Versions tend to work outside of this range. So they user may be okay with the new Helm / old Kubernetes (it just isn't "guaranteed").

So I don't think it will "force" users to upgrade Kubernetes versions. The only situation where an upgrade would be required, is if a user does want a new chart version, which requires a new Helm version. And the chart or Helm is incompatible with the old Kubernetes version.

gjenkins8 · 2025-02-09T03:58:06Z

Do we have some other new chart features so that we should have a new chart version? We are currently on chart apiVersion v2.

Plugins would be the example that comes to mind.

edit: there is also https://github.com/helm/community/blob/main/hips/hip-0015.md, where ideally annotations would not be used

Signed-off-by: George Jenkins <[email protected]>

gjenkins8 · 2025-02-09T18:59:34Z

We could of course, also mark chart API version 2 as closed (given its age, and intended stability). A minimumHelmVersion field should/would still be applicable to a charts API version 3 I think

That said, if we did want to introduce plugins to chart API version 2, the concerns mentioned in this HIP around forward compatibility would apply

pull-request-size bot added the size/L label Nov 19, 2024

gjenkins8 changed the title ~~Forward compatibility HIP~~ HIP: Forward compatibility: Chart.yaml minimumHelmVersion Nov 19, 2024

gjenkins8 force-pushed the hip_forward_compat branch 2 times, most recently from dcd384b to 47c276b Compare November 19, 2024 06:53

marckhouzam reviewed Nov 19, 2024

View reviewed changes

hips/hip-XXXX.md Outdated Show resolved Hide resolved

hips/hip-XXXX.md Outdated Show resolved Hide resolved

hips/hip-XXXX.md Outdated Show resolved Hide resolved

hips/hip-XXXX.md Outdated Show resolved Hide resolved

gjenkins8 changed the title ~~HIP: Forward compatibility: Chart.yaml minimumHelmVersion~~ HIP: Forward compatibility: Chart.yaml minimumHelmVersion field Nov 19, 2024

gjenkins8 changed the title ~~HIP: Forward compatibility: Chart.yaml minimumHelmVersion field~~ H4HIP: Forward compatibility: Chart.yaml minimumHelmVersion field Nov 19, 2024

mattfarina reviewed Nov 21, 2024

View reviewed changes

sabre1041 reviewed Nov 24, 2024

View reviewed changes

gjenkins8 mentioned this pull request Nov 24, 2024

H4HIP: Default to strict Chart.yaml loading #371

Open

godhanipayal reviewed Jan 30, 2025

View reviewed changes

Forward compatibility HIP

b21a1c1

Signed-off-by: George Jenkins <[email protected]>

gjenkins8 force-pushed the hip_forward_compat branch from 783a114 to b21a1c1 Compare February 9, 2025 16:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

H4HIP: Forward compatibility: `Chart.yaml` `minimumHelmVersion` field #370

H4HIP: Forward compatibility: `Chart.yaml` `minimumHelmVersion` field #370

gjenkins8 commented Nov 19, 2024

marckhouzam left a comment

mattfarina left a comment

mattfarina Nov 21, 2024

gjenkins8 Nov 21, 2024 •

edited

Loading

sabre1041 Nov 24, 2024

gjenkins8 Nov 24, 2024

mattfarina Nov 21, 2024

mattfarina Nov 21, 2024

gjenkins8 Nov 24, 2024

mattfarina Nov 21, 2024

gjenkins8 Feb 9, 2025

mattfarina Nov 21, 2024

gjenkins8 Nov 24, 2024

sabre1041 left a comment

sabre1041 Nov 24, 2024

sabre1041 Nov 24, 2024

gjenkins8 Feb 9, 2025

sabre1041 Nov 24, 2024

gjenkins8 Nov 29, 2024

godhanipayal Jan 30, 2025

gjenkins8 Feb 9, 2025

gjenkins8 commented Feb 9, 2025 •

edited

Loading

gjenkins8 commented Feb 9, 2025 •

edited

Loading


		When loading a chart, iff the `minimumHelmVersion` exists, Helm will verify its version exceeds or equals the version constraint specified in the field.

		When "strict" loading of `Chart.yaml` is used: ie. the loading of `Chart.yaml` errors when unknown fields are present, Helm will first 'peek' into `Chart.yaml` and extract the `minimumHelmVersion` field (iff it exists) and perform the version constraint check at this point.


		## Abstract

		This HIP proposes supporting a `minimumHelmVersion` field in `Chart.yaml`.

H4HIP: Forward compatibility: Chart.yaml minimumHelmVersion field #370

Are you sure you want to change the base?

H4HIP: Forward compatibility: Chart.yaml minimumHelmVersion field #370

Conversation

gjenkins8 commented Nov 19, 2024

marckhouzam left a comment

Choose a reason for hiding this comment

mattfarina left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gjenkins8 Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sabre1041 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gjenkins8 commented Feb 9, 2025 • edited Loading

gjenkins8 commented Feb 9, 2025 • edited Loading

H4HIP: Forward compatibility: `Chart.yaml` `minimumHelmVersion` field #370

H4HIP: Forward compatibility: `Chart.yaml` `minimumHelmVersion` field #370

gjenkins8 Nov 21, 2024 •

edited

Loading

gjenkins8 commented Feb 9, 2025 •

edited

Loading

gjenkins8 commented Feb 9, 2025 •

edited

Loading